-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allennlp Predictor class that mimics gec_model #6
Conversation
…y gec_model, get_token_action removed and placed in model.
There's a bug in here somewhere though because the output isn't exactly the same. I suspect there's an off by one error happening when the indices for edit operations are generated.
…l words and corrected words to the output.
… errors are identified. Fixing offsets in light of addition of START_TOKEN to front.
Everything works now except that the text is getting lowercased somewhere.
…offsets when creating the target (corrected sentence). Copying get_token_action method almost verbatim instead of using something more clever.
…. This is identical to the output for gec_model!
Sorry! I forgot to add Nitin and Damien to the list of reviewers somehow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not that familiar with this codebase so I am going to approve based on what I see and let @Frost45 and @damien2012eng do a more careful review.
Great work, @ksteimel ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is amazing work @ksteimel! Thank you for taking this on! Just a few minor comments.
Co-authored-by: Sanjna Kashyap <[email protected]>
Sorry for the large PR. These changes introduce a gec_predictor class that is able to mimic the behavior of gec_model but in a way that follows the allennlp api.
This does handle multi-iteration prediction. However, gec_model had an option for ensemble prediction and that is currently not handled here.
I have also created a model archive that can be used with this new predictor. You can see the config file for this in test_fixtures/roberta_model. The weights file is downloaded the appropriate location and shared between the predictor and gec_model.. I can tar the folder up and then add it to a public s3 bucket after this PR is approved.